fix(cpp): TS_2DIFF float/double maxPointNumber once per page (fixes #910) - #901

Thanks for tracking this down.

The root-cause analysis is clear, and the new implementation correctly handles Java-compatible prefixes, including overflow prefixes and reads spanning multiple segments.

I found one blocking compatibility issue, though: routing FLOAT/DOUBLE batch reads through the scalar decoder regresses legacy raw segments. The scalar prefix detector can misclassify a valid raw header, after which the decoder gets an invalid bit_width_ and spins at end-of-input.

I reproduced this for both FLOAT and DOUBLE by encoding 129 sequential raw bit patterns with IntTS2DIFFEncoder / LongTS2DIFFEncoder, then reading them in small batches through the corresponding floating-point decoder. The PR head hangs, while the parent implementation completes successfully.

Could we preserve the integer batch path for legacy raw segments, or make the prefix detection unambiguous before switching to the scalar path? It would also be good to add legacy raw batch regression tests for both types.

Hi @ColinLeeo, thanks for the thorough review and the reproduction steps — they made this straightforward to chase down. I've pushed 1ef5e94 addressing all three points.

Root cause confirmed. Your repro hangs exactly as described: the per-block heuristic (looks_like_ts2diff_header) only validated a single misaligned block header (wi/bw range check). A legacy raw block with wi = 0 (or bit_width = 0, i.e. any constant-value block) passes that probe with all-zero bytes, gets misread as a maxPointNumber prefix, and the desync cascades into the end-of-input spin.

Fix — unambiguous prefix detection (your option 2). The layout is now decided once per page by scan_java_float_double_page(), which parses the entire remaining stream with the Java segment grammar: [overflow flag][count][bitmaps][mpn] block+, with field-range validation, Σ(wi+1) == bitmap count for overflow segments, and exact whole-stream exhaustion. A page only counts as Java-compatible when the grammar consumes it exactly. A real legacy raw page fails this immediately: after the varint tag eats the leading 0x00, the misaligned write_index probe reads >= 0x100 and is rejected. The detected prefix offsets are recorded and the decoder only consumes prefixes at those offsets, which also fixes value semantics for Java single-segment multi-block pages the per-block heuristic couldn't represent.

Legacy raw batch path preserved (your option 1). Legacy raw pages route through the integer SIMD batch decoder + bit-cast, exactly the parent-commit behavior; Java pages take the segment-aware scalar path.

Regression tests. Added for both FLOAT and DOUBLE:

• ReadBatchFloatLegacyRawSegments / ReadBatchDoubleLegacyRawSegments — 129 values via IntTS2DIFFEncoder/LongTS2DIFFEncoder (128-value constant block + trailing change, hitting both the bit_width = 0 and wi = 0 misclassification patterns), read in small batches of 16
• ReadFloatLegacyRawScalar / ReadDoubleLegacyRawScalar — scalar path across the block boundary
• LegacyRawBatchThenScalarReads — mixed batch/scalar reads on one page

Also hardened read_long() to bail out when the stream is exhausted with bits still owed, so no residual misconfiguration can loop forever.

One incidental fix this surfaced: ByteStream::check_space() skipped a page when set_read_pos() parked the cursor at a page boundary (it blindly followed read_page_->next_, yielding E_OUT_OF_RANGE on the next read). The scan-based detection depends on position restore, so I fixed it to recompute the page from the head.

Full C++ suite (757 tests) passes, and clang-format --dry-run --Werror is clean. Happy to adjust if you'd prefer a different split.

Thanks for the review. I have pushed a66a679 which fixes the spotless clang-format violations flagged by CI (ts2diff_decoder.h and ts2diff_codec_test.cc).

The CI runs for this new commit are currently waiting for approval (action_required) — could you approve the workflows so they can re-run?

Happy to address any remaining feedback on the legacy-raw compatibility path.

One more thought regarding the compatibility design: do we really need to support the historical C++ TS_2DIFF layout?
Since Java implementation is the reference for TsFile format compatibility, I think the previous C++ behavior (writing maxPointNumber for every block) should be considered as a C++ implementation bug rather than a legacy format that needs to be preserved.
Supporting this old layout introduces additional format detection logic and ambiguity in the decoder. It may make the implementation harder to maintain.
Would it be possible to simplify this PR by:

• making C++ writer follow the Java layout exactly;
• making C++ reader support the Java layout only;
• removing the old C++ layout compatibility path?

Then the regression tests can focus on Java ↔ C++ interoperability.

Hi, @kkzi

Clarification of the FLOAT/DOUBLE TS_2DIFF Format and the Direction of This Fix

TL;DR

• The Java FLOAT/DOUBLE + TS_2DIFF layout is the canonical layout for cross-language compatibility. The C++ writer and reader use this format as their compatibility boundary.
• The compatibility scope does not include the raw bit-cast layout produced by the earlier C++ FLOAT/DOUBLE + TS_2DIFF writer.

# C++ layout introduced by #796
[overflow marker 1][blockValueCount 1][bitmap(s) 1][maxPointNumber][block 1]
[overflow marker 2][blockValueCount 2][bitmap(s) 2][maxPointNumber][block 2]
[overflow marker 3][blockValueCount 3][bitmap(s) 3][maxPointNumber][block 3]

# C++ layout in the current PR
[overflow marker 1][blockValueCount 1][bitmap(s) 1][maxPointNumber][block 1]
[overflow marker 2][blockValueCount 2][bitmap(s) 2]                [block 2]
[overflow marker 3][blockValueCount 3][bitmap(s) 3]                [block 3]

# Canonical Java layout
[overflow marker][pageValueCount][page-wide bitmap(s)]
[maxPointNumber][block 1][block 2][block 3]

I reviewed the Java and C++ encoder/decoder implementations and their history again. In the earlier discussion, I mixed together the official Java FLOAT/DOUBLE format, the raw format produced by the early C++ writer, and the per-block prefix format produced by the later C++ writer. I apologize for the confusion. The sections below describe these formats separately and note the remaining boundaries in the current code.

1. How Java Handles FLOAT/DOUBLE TS_2DIFF

TS_2DIFF itself encodes integers. Java adds a FLOAT/DOUBLE wrapper around it:

FLOAT  -> int32 -> IntDeltaEncoder
DOUBLE -> int64 -> LongDeltaEncoder

Suppose maxPointNumber = 2, so maxPointValue = 100. Java handles each value in one of three ways:

To distinguish these cases, Java writes one or two page-wide bitmaps when needed.

One detail is that Java uses Float.floatToIntBits / Double.doubleToLongBits. Regular values and Infinity retain their corresponding IEEE 754 bit patterns, while NaN is converted to Java's canonical NaN bit pattern.

The Java page layout has three forms:

# Every value can be scaled normally
[maxPointNumber]
[TS_2DIFF block 1][TS_2DIFF block 2]...

# At least one scaled value overflows, but no original value overflows
[Integer.MAX_VALUE]
[pageValueCount]
[page-wide scaled-value bitmap]
[maxPointNumber]
[TS_2DIFF block 1][TS_2DIFF block 2]...

# At least one value is stored as its raw IEEE 754 bits
[Integer.MAX_VALUE - 1]
[pageValueCount]
[page-wide scaled-value bitmap]
[page-wide original-value bitmap]
[maxPointNumber]
[TS_2DIFF block 1][TS_2DIFF block 2]...

The key points are:

• maxPointNumber appears once per page.
• The overflow flags and bitmaps are also page-level metadata.
• Each bitmap covers the entire page rather than one TS_2DIFF block.
• The metadata is followed by a continuous sequence of integer TS_2DIFF blocks, with no additional FLOAT/DOUBLE prefix between blocks.

2. The Previously Mentioned Raw TS_2DIFF Path

Before #796, the C++ FloatTS2DIFFEncoder / DoubleTS2DIFFEncoder interpreted each floating-point value as an integer of the same width and then applied integer TS_2DIFF:

float IEEE bits -> int32 -> integer TS_2DIFF -> int32 -> float IEEE bits

double IEEE bits -> int64 -> integer TS_2DIFF -> int64 -> double IEEE bits

This approach preserves the floating-point bits losslessly. It is not the Java FLOAT/DOUBLE TS_2DIFF format, but it was once the official C++ writer output when FLOAT/DOUBLE + TS_2DIFF was selected explicitly.

My earlier regression test used IntTS2DIFFEncoder / LongTS2DIFFEncoder to encode the same IEEE bit patterns, producing a payload equivalent to this earlier writer path. The test demonstrated that the new prefix detection could misclassify a historical raw block as a Java floating-point prefix and eventually cause the decoder to hang.

The current writer no longer produces the raw layout, while the reader's handling of it represents compatibility logic for historical C++ files.

The raw layout was private to the early C++ implementation and was never supported by the Java reader, so it is not part of the cross-language TsFile format. Detecting the raw and Java layouts from the input bytes retains this historical behavior in the decoder state machine and also introduces format ambiguity.

From the perspective of format boundaries and implementation complexity, I prefer focusing the C++ decoder on the canonical Java layout and returning a format error for the earlier raw layout. This simplifies prefix handling and avoids continuing to decode after a misclassification. If the community wants to retain support for early C++ files, that behavior can be discussed separately with a more explicit format identifier.

3. C++ Writer Layout After the Java-Style Wrapper Was Introduced

#796 changed C++ FLOAT/DOUBLE TS_2DIFF from raw bit-casting to Java-style scaling, maxPointNumber, and overflow bitmaps, followed by integer TS_2DIFF.

However, the integer encoder triggers a block flush after accumulating 129 values, and FloatTS2DIFFEncoder::flush() also writes the floating-point wrapper metadata.

As a result, this version of the C++ writer produces a per-block layout. For a page containing three blocks where every block has an overflow value, the layout is:

[overflow marker 1][blockValueCount 1][bitmap(s) 1][maxPointNumber][block 1]
[overflow marker 2][blockValueCount 2][bitmap(s) 2][maxPointNumber][block 2]
[overflow marker 3][blockValueCount 3][bitmap(s) 3][maxPointNumber][block 3]

A block without overflow uses:

[maxPointNumber][block]

This layout results from the integer block flush and the FLOAT/DOUBLE wrapper flush sharing the same flush() method. After every 129 values, the integer encoder invokes the virtual flush() method. During that call, the floating-point encoder generates the bitmap for the current block, writes it, and clears underflow_flags_. The next block collects a new set of flags and generates another bitmap.

The difference from the Java page-wide layout is the metadata scope. This discussion uses the Java layout as the format baseline: the target C++ encoder/decoder layout has page-wide metadata, while the earlier C++ per-block layout is outside the compatibility scope.

The default encoding for FLOAT/DOUBLE is GORILLA. An explicit TS_2DIFF schema choice still selects this C++ writer path.

4. Format Differences That Remain in the Current PR

This PR uses max_point_number_saved_ to avoid writing maxPointNumber repeatedly for every block. A multi-block page without overflow now has this layout:

[maxPointNumber][block 1][block 2]...

This part matches Java.

One remaining difference concerns metadata scope: underflow_flags_ is still cleared after each block flush, and each overflow bitmap is still generated per block. When overflow occurs, C++ therefore continues to write per-block metadata rather than Java's page-wide metadata.

The expanded comparison of the three layouts appears in the TL;DR.

The current PR removes the repeated maxPointNumber before later blocks, but the bitmaps remain per-block, so the resulting layout still differs from Java.

The decoder follows the same per-block model. When entering a later block, it clears the bitmap and resets segment_pos_ to 0. When reading a multi-block overflow page produced by Java, it loses the page-wide bitmap state after the first block.

Another related boundary is maxPointNumber = 0. The Java encoder supports this configuration, for which the first byte of a canonical page is 0x00. The current prefix detection treats a leading 0x00 byte as a legacy raw payload. For a simple payload whose first value is 1.0, the C++ decoder returns E_OK but produces approximately 2.35099e-38.

For canonical Java format compatibility, the current PR has completed the change that writes maxPointNumber once per page. The overflow metadata and decoder bitmap state remain block-scoped.

5. One Possible Implementation Direction

The encoder could collect floating-point conversion state across the entire page and generate the flags, bitmaps, and maxPointNumber once during the page flush, followed by a continuous sequence of integer TS_2DIFF blocks.

The decoder could parse the FLOAT/DOUBLE metadata once at the beginning of the page and retain the bitmap and current value position throughout the page. When it enters a new integer block, it would continue using the same page-wide bitmap and position.

The C++ implementation uses the canonical Java layout as its format baseline. The automatic detection and fallback logic that distinguishes the raw layout, the earlier C++ per-block layout, and the Java layout can be simplified at the same time. The decoder then maintains only the Java format state machine, while other inputs return a format error.

The existing batch decoder remains reusable:

1. Parse the page-level FLOAT/DOUBLE metadata once
2. Decode the integer TS_2DIFF blocks with read_batch_int32/read_batch_int64
3. Look up the bitmap using the page-wide position
4. Convert the integers in the batch to FLOAT/DOUBLE

Per-value bitmap checks and numeric conversion remain, while the main bit unpacking, delta reconstruction, and SIMD batch paths can still be reused. FLOAT/DOUBLE batch reads can also reuse the main flow of the integer batch decoder.

6. Additional Validation for the Current PR

The existing Java/C++ compatibility test can be reused. Relevant locations include:

• Java: java/tsfile/src/test/java/org/apache/tsfile/compatibility/TableModelEncodingCompressionCompatibilityTest.java
• C++: cpp/test/reader/table_view/table_model_encoding_compression_compatibility_test.cc
• Workflow: .github/workflows/compatibility-test.yml

At present, Java's buildMatrix() and C++'s BuildMatrix() cover only CHIMP, RLBE, and CAMEL. They do not include FLOAT/DOUBLE + TS_2DIFF. Both sides also write only 32 points by default, so the tests do not cross the boundary of a TS_2DIFF block containing 129 values.

Two additions can extend this coverage:

encoding matrix += FLOAT + TS_2DIFF
encoding matrix += DOUBLE + TS_2DIFF
rowCount = 300  # apply to all compatibility cases

In other words, the matrices on both sides can include FLOAT/DOUBLE + TS_2DIFF, while the number of written points for every existing compatibility case can be increased to 300. These cases can continue using the existing fixture generation, manifest, and bidirectional validation flow.

One detail is worth noting: the current compatibility test uses exact-bit comparisons for FLOAT/DOUBLE, while TS_2DIFF applies a fixed-point conversion based on maxPointNumber. Values such as -0.0 and pi in the existing data may not retain their original bits after TS_2DIFF encoding. The new combinations can either use data that is restored exactly after scaling or reflect the TS_2DIFF conversion rules in their expected values.

To cover the page-wide bitmap across blocks, the compatibility cases can use rowCount = 300 and include scaled-overflow data among those 300 values. This crosses the 129-value block boundary and exercises both Java page-wide bitmap reads and C++ multi-block writer output.

A Java fixture with maxPointNumber = 0 can also cover the prefix boundary. It verifies that a canonical page beginning with 0x00 enters the correct parsing path.

I plan to cover a more complete cross-language compatibility matrix in a separate follow-up PR, including parameterized row counts, data types, encodings, and compression combinations. If increasing the row count reveals compatibility issues in other combinations, those can be tracked in separate issues.

Hi @ColinLeeo, thanks for the detailed 8/24 clarification — the format baseline and the section-by-section layout analysis made the rework straightforward to scope. The branch is rebased onto develop (the compatibility infrastructure from #905 is now available) and implements the direction you outlined.

What changed

Encoder (page-wide metadata). The integer encoder's automatic 129-value block flush now emits a plain integer block into an internal page buffer; overflow flags and buffered blocks survive the block boundary. The page-seal flush emits the page metadata once — [overflow marker][pageValueCount][page-wide bitmap(s)][maxPointNumber] followed by the continuous block sequence — matching the canonical Java layout in all three forms. The per-block wrapper metadata is gone.

Bit width. While validating against the Java hex golden tests I found the old width computation (max - min) also wraps negative when raw deltas wrap the signed type (e.g. adjacent raw IEEE bit patterns), silently writing bit width 0 and discarding every delta. The encoder now widens each rebased delta individually, mirroring calculateBitWidthsForDeltaBlockBuffer.

Decoder (single grammar, page-wide state). All layout sniffing is removed (scan/looks_like_ts2diff_header/is_legacy_raw_/per-segment maxPointNumber handling). Page metadata is parsed exactly once per page; the bitmaps and the page position survive block transitions, so Java multi-block overflow pages decode correctly. maxPointNumber = 0 (page starting with 0x00) is a valid Form 1 page. The raw bit-cast layout and the pre-#910 per-segment layout now fail fast with E_TSFILE_CORRUPTED: block headers are validated (write_index ∈ [0,128], width in range) and a truncated header fails instead of reusing stale state, which was the root of the end-of-input spin — batch readers can no longer loop on out-of-format input. The legacy regression tests were rewritten to assert fail-fast behavior.

Batch reads. FLOAT/DOUBLE read_batch reuses the integer batch decoder (SIMD fast path) and applies the page-wide bitmaps afterwards per page position, as you sketched in §5.

Tests. Both matrices now include FLOAT + TS_2DIFF and DOUBLE + TS_2DIFF and every case writes 300 rows (129+129+42). The value set covers scaled, scale-overflow (1.5e9f / 1e18, reachable only at mpn > 0), and raw-bit forms (canonical NaN, ±Inf). Every chosen value restores identically at maxPointNumber 0 and 2, so the validating reader never needs to know which writer produced a file; expectations are computed from the encoding rules rather than the input bits. A wire-format contract document derived from the Java reference implementations is included (cpp/docs/ts2diff-float-double-wire-format.md).

Verification

All four compatibility directions on the extended matrix pass bit-exactly (30 cases each, LZMA2 excluded locally — see below), including FLOAT/DOUBLE TS_2DIFF across the block boundary with page-wide bitmaps. Full C++ suite: 787 tests, 784 pass / 3 skipped (env-gated compat fixtures). Java reads Java's 45 fixtures (LZMA2 included) cleanly.

Two things to flag

1. LZMA2 on Windows/MSVC Debug: write_table fails with E_INVALID_ARG for any encoding when LZMA2 is selected (reproducible with CHIMP + LZMA2 on develop's feat(cpp): refactor dependency sourcing and extend codec compatibility #905 without my changes), so the 15 LZMA2 cases fail locally on the C++ side. Java handles LZMA2 fine, which suggests a C++-side (possibly MSVC-specific) issue in feat(cpp): refactor dependency sourcing and extend codec compatibility #905. Happy to file a separate issue with the repro if useful.
2. C++ default max_point_number_ = 2 vs Java's Ts2Diff builder hard-coded 0: the value is self-describing in the stream so interop is unaffected, but since "Java is the format baseline" I can align the default to 0 in this PR or a follow-up, whichever you prefer.

The commits are structured as: matrix + contract doc, encoder rework, decoder rework. Glad to restructure or address anything else.

Follow-up on point 2: ff7bfee aligns the C++ FloatTS2DIFFEncoder/DoubleTS2DIFFEncoder default max_point_number to the Java Ts2Diff builder value (0), so pages written by either implementation are now byte-identical by default.

Notes from the change:

• The scale-overflow form (Form 2) is unreachable at mpn = 0 — the scaled product equals the value itself, so any overflow is a value overflow taking the raw-bits path. The wire-format doc records this; the compatibility value set keeps the former Form-2 entries (1.5e9f / 1e18), which now exercise the raw-bits path, and expectations are unchanged (the set is mpn-agnostic by construction).
• The Java hex goldens were regenerated with mpn = 0.
• Tests that counted 0x02 prefix bytes were replaced with a structural page walker (parse the metadata once, then verify a continuous well-formed block stream) — byte counting cannot distinguish the 0x00 mpn byte from block-header high bytes.
• Round-trip ramp data was integerized so expectations hold under the default mpv = 1.

Re-verified: full C++ suite 784/787 pass, and all four compatibility directions remain green on the 30 non-LZMA2 cases.